Adding ChangeInstrument op #1005

wan9c9 · 2022-03-23T11:17:48Z

Description

The ChangeInstrument allows using the features of another instrument.

Motivation and Context

How Has This Been Tested?

Pass the test by running: pytest qlib/tests/test_all_pipeline.py under upper directory of qlib.
If you are adding a new feature, test on your own test scripts.

Screenshots of Test Results (if appropriate):

Pipeline test:
Your own tests:

Types of changes

Fix bugs
Add new feature
Update documentation

Adding Change instrument OP. This op allows one to use features of a different instrument.

update parse_field to accommodate ChangeInstrument

ghost · 2022-03-23T11:18:00Z

All CLA requirements met.

you-n-g · 2022-04-02T08:00:46Z

@wan9c9

Please fix the CI

qlib/utils/__init__.py

qlib/data/ops.py

you-n-g · 2022-04-11T08:14:18Z

qlib/data/ops.py

+    def __str__(self):
+        return "{}({},{})".format(type(self).__name__, self.instrument, self.feature)
+
+    def load(self, instrument, start_index, end_index, freq):


super().load(self, instrument, start_index, end_index, *args)
Will it better to call the method from super class directly?

Thanks for the comments. I was trying to use self.instrument as part of the cache key in the load function, since this op is likely used to add some features from frequently used instruments such as market index or sector index. In this case, rewriting the load function with self.instrument as part of cache args enables the caching is done with the changed instrument, rather than the original instrument, this should be more efficient than using super().load(self, instrument, start_index, end_index, *args).

Another way to achieve the same caching performance may be to call super().load(self, self.instrument, start_index, end_index, *args). This should do the trick without changing the _load_internal function, right?

Please let me know how you think and I can revise it. Thanks!

Yes.
I think this is a more concise way to implement.

Another way to achieve the same caching performance may be to call super().load(self, self.instrument, start_index, end_index, *args). This should do the trick without changing the _load_internal function, right?

@wan9c9
Hi, did you encounter any problems when implementing the concise version ?

you-n-g · 2022-04-11T08:15:32Z

qlib/data/ops.py

@@ -32,6 +32,91 @@

 np.seterr(invalid="ignore")

+
+#################### Change instrument ########################


This duplicates with the docs string.
I think we can keep only the one in the docstring.

Yes, sure. I wrote the first paragraph to motivate the class, and it should be fine to keep only those in the doc string and I'll update that.

Hi, @wan9c9 did you fix this ?
Thanks

you-n-g · 2022-04-11T08:16:59Z

Hi, @wan9c9
Your implementation is really smart! I think it will benefit a lot of users.
I add several comments just now. It will be great if you can refine it :)

you-n-g · 2022-04-15T10:03:02Z

Hi, @wan9c9
I've replied to your questions about the issues.
Looking forward to your reply :).

BTW, please fix the error reported by CI

Propose test

you-n-g · 2022-06-06T10:55:32Z

@wan9c9 did you encounter any problems when fix the remained issues in the comments?

wan9c9 · 2022-06-09T22:49:59Z

@you-n-g, sorry for the delays, I'm busy with other stuff and couldn't commit the revision as I also messed up the code. I did wrote some tests, which couldn't pass the test. The test code are show below.

class TestOperatorDataSetting(TestOperatorData):
def test_setting(self):

    def test_case(instruments, queries, note=None):
        if note:
            print(note)
        print(f"checking {instruments} with queries {queries}")
        df = D.features(instruments, queries)
        print(df)
        return df

    test_case(["SH600519"], ["ChangeInstrument('SH000300', $close)"], "get market index close")
    test_case(["SH600519"], ["ChangeInstrument('SH000300', Feature('close')/Ref(Feature('close'),1) -1)"],
              "get market index return with Feature")
    test_case(["SH600519"], ["ChangeInstrument('SH000300', $close/Ref($close,1) -1)"],
              "get market index return with expression")
    test_case(["SH600519"], ["($close/Ref($close,1) -1) - ChangeInstrument('SH000300', $close/Ref($close,1) -1)"],
              "get excess return with expression with beta=1")

    ret = "Feature('close') / Ref(Feature('close'), 1) - 1"
    benchmark = 'SH000300'
    n_period = 252
    marketRet = f"ChangeInstrument(\'{benchmark}\', Feature('close') / Ref(Feature('close'), 1) - 1)"
    marketVar = f"ChangeInstrument(\'{benchmark}\', Var({marketRet}, {n_period}))"
    beta = f"Cov({ret}, {marketRet}, {n_period}) / {marketVar}"
    excess_return = f"{ret} - {beta}*({marketRet})"
    fields = ["Feature('close')", f"ChangeInstrument(\'{benchmark}\', Feature('close'))", ret, marketRet, beta, excess_return]
    test_case(["SH600519"],fields[5:],
              "get market beta and excess_return with estimated beta")

    # All the queries above passed

    """
    the code below did not pass if  the following lines are not added to qlib/utils/__init__.py
    but will pass if added.
    # for ChangeInstrument
    field = re.sub(r"Operators.ChangeInstrument\((\w+),",
                   rf'Operators.ChangeInstrument("\1",', field)
    """

    from qlib.data.dataset.loader import QlibDataLoader
    from qlib.data.ops import Feature, Ref, ChangeInstrument, Var, Cov
    instrument = "SH600519"
    ret = Feature('close') / Ref(Feature('close'), 1) - 1
    benchmark = 'SH000300'
    n_period = 252
    marketRet = ChangeInstrument(benchmark, Feature('close') / Ref(Feature('close'), 1) - 1)
    marketVar = ChangeInstrument(benchmark, Var(marketRet, n_period))
    beta = Cov(ret, marketRet, n_period) / marketVar
    fields = [Feature('close'), ChangeInstrument(benchmark, Feature('close')),
              ret, marketRet,
              beta, ret - beta*marketRet]
    names = ['close', 'marketClose', 'ret', 'marketRet', f"beta_{n_period}","excess_return"]
    data_loader_config = {"feature": (fields,
                                      names
                                      )
                          }
    data_loader = QlibDataLoader(config=data_loader_config)
    df = data_loader.load(instruments=[instrument])#, start_time=start_time)
    print(df)

    #test_case(["SH600519"],fields,
              #"get market beta and excess_return with estimated beta")

if name == "main":
unittest.main()

Could you please take a look?

Thanks!

you-n-g · 2022-06-14T14:07:15Z

@wan9c9 I added your test case here and fix a bug. Please merge it
https://github.com/wan9c9/qlib/pull/4wan9c9
Is the problem solved? Do you have other issues when fixing the remaining comments in this PR?

wan9c9 · 2022-06-14T16:48:22Z

@you-n-g, looks like the link https://github.com/wan9c9/qlib/pull/4wan9c9 doesn't work. Can you please send a valid link and I'll double check? Thanks!

you-n-g · 2022-06-15T02:23:52Z

@you-n-g, looks like the link https://github.com/wan9c9/qlib/pull/4wan9c9 doesn't work. Can you please send a valid link and I'll double check? Thanks!

Sorry I mixed your ID and the link. Here is the right link wan9c9#4
@wan9c9

Add test case and fix bug

you-n-g · 2022-07-04T00:45:21Z

@wan9c9 It looks great now!
Thanks for your contribution!

* add ChangeInstrument to ops Adding Change instrument OP. This op allows one to use features of a different instrument. * Update __init__.py update parse_field to accommodate ChangeInstrument * Propose test * Add test case and fix bug * Update ops.py * Update ops.py * simplify the operator further * implement abstract method * fix arg bug * clean test Co-authored-by: Young <afe.young@gmail.com> Co-authored-by: you-n-g <you-n-g@users.noreply.github.com>

wan9c9 added 2 commits March 23, 2022 07:07

add ChangeInstrument to ops

9b916ec

Adding Change instrument OP. This op allows one to use features of a different instrument.

Update __init__.py

3d3a10c

update parse_field to accommodate ChangeInstrument

you-n-g reviewed Apr 11, 2022

View reviewed changes

Propose test

097c4d1

you-n-g force-pushed the main branch from e80c52d to c5cf8fb Compare April 29, 2022 09:21

Merge pull request #3 from you-n-g/propose_test

bd44677

Propose test

Add test case and fix bug

6c28bfa

wan9c9 and others added 8 commits June 16, 2022 06:56

Merge pull request #4 from you-n-g/help_fix_bug

202064a

Add test case and fix bug

Update ops.py

a936f42

Update ops.py

e6ebe03

Merge remote-tracking branch 'origin/main' into HEAD

9c545c2

simplify the operator further

b81ff08

implement abstract method

2a85e56

fix arg bug

8ed6eac

clean test

5ed53b4

you-n-g merged commit 3db2245 into microsoft:main Jul 4, 2022

you-n-g added the enhancement New feature or request label Dec 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding ChangeInstrument op #1005

Adding ChangeInstrument op #1005

wan9c9 commented Mar 23, 2022

ghost commented Mar 23, 2022 •

edited by ghost

Loading

you-n-g commented Apr 2, 2022

you-n-g Apr 11, 2022

wan9c9 Apr 12, 2022

wan9c9 Apr 12, 2022

you-n-g Apr 15, 2022

you-n-g Jun 6, 2022

you-n-g Apr 11, 2022

wan9c9 Apr 12, 2022

you-n-g Jun 6, 2022

you-n-g commented Apr 11, 2022

you-n-g commented Apr 15, 2022

you-n-g commented Jun 6, 2022

wan9c9 commented Jun 9, 2022

you-n-g commented Jun 14, 2022 •

edited

Loading

wan9c9 commented Jun 14, 2022

you-n-g commented Jun 15, 2022 •

edited

Loading

you-n-g commented Jul 4, 2022

		@@ -32,6 +32,91 @@

		np.seterr(invalid="ignore")


		#################### Change instrument ########################

Adding ChangeInstrument op #1005

Adding ChangeInstrument op #1005

Conversation

wan9c9 commented Mar 23, 2022

Description

Motivation and Context

How Has This Been Tested?

Screenshots of Test Results (if appropriate):

Types of changes

ghost commented Mar 23, 2022 • edited by ghost Loading

you-n-g commented Apr 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

you-n-g commented Apr 11, 2022

you-n-g commented Apr 15, 2022

you-n-g commented Jun 6, 2022

wan9c9 commented Jun 9, 2022

you-n-g commented Jun 14, 2022 • edited Loading

wan9c9 commented Jun 14, 2022

you-n-g commented Jun 15, 2022 • edited Loading

you-n-g commented Jul 4, 2022

ghost commented Mar 23, 2022 •

edited by ghost

Loading

you-n-g commented Jun 14, 2022 •

edited

Loading

you-n-g commented Jun 15, 2022 •

edited

Loading